-
Notifications
You must be signed in to change notification settings - Fork 30
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
partition fix #47
partition fix #47
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@fivetran-jamie this PR looks great. I have one small request for rebasing the branch and we will include this in a breaking change for v0.7.0
.
However, I set the review for requesting changes, but in actuality I would really just like for you to walk @fivetran-reneeli and I through the process of how this will help cleanup the zendesk__ticket_field_history
model (ie. stop producing records after so many days of being closed). We can do this during standup tomorrow.
We can pick a ticket or two tomorrow and see with these changes how if the ticket is closed for X days then it will stop creating records, and also how it will impact records that are still open. With all the changes that will be applied to this package I want our team to get a better understanding of how all these moving pieces will work together!
Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great!! Thanks so much for working on this @fivetran-jamie! 🥇
Are you a current Fivetran customer?
work at fivetran
What change(s) does this PR introduce?
Does this PR introduce a breaking change?
yes in that now ticket field histories are capped at the close date, so like running a full refresh with this code will remove some rows that existed before (of post-closure tickets). no in that that may not be considered "breaking" since the tickets are closed and also in that the
ticket_field_history_extension_months
variable can be used to persist this behaviorthoughts? @fivetran-joemarkiewicz @fivetran-reneeli
Is this PR in response to a previously created Issue
How did you test the PR changes?
Select which warehouse(s) were used to test the PR
Provide an emoji that best describes your current mood
🐈
Feedback
We are so excited you decided to contribute to the Fivetran community dbt package! We continue to work to improve the packages and would greatly appreciate your feedback on our existing dbt packages or what you'd like to see next.